fix(mobile): apply exif orientation to android raw photos#29337
fix(mobile): apply exif orientation to android raw photos#29337santoshakil wants to merge 2 commits into
Conversation
android's ImageDecoder/loadThumbnail (API 29+) skip the EXIF orientation tag for raw files like DNG, so portrait raw shots showed up sideways in the grid and viewer. jpeg/heic were fine since those decoders rotate on their own. read the orientation tag and rotate the decoded raw bitmap to match, on the same background pool so the ui doesn't jank. the load-original full-res decode is sampled down first so the rotate copy can't OOM on high-mp sensors. raw only, jpeg/heic and pre-29 paths unchanged.
|
📱 Android release APK (universal) — Download: https://github.com/immich-app/immich/actions/runs/28241443757/artifacts/7906911071 QR codeInstalls as a separate app (applicationId |
|
Surely this can be done in-place rather than allocating a separate bitmap? That seems hugely wasteful. |
fair point. for thumbs and the preview the bitmap is small so it's cheap, and load original is capped at around 24mp anyway, so the double alloc only really hits big raws. but yeah, we copy into a native buffer right after so the separate bitmap is redundant. i can fold the rotate into that copy and skip the second bitmap. only thing, the 90 and 270 case is a transpose and doing it on the jvm might be slower than the native createBitmap, so let me bench both and let's see which one wins. |
|
@mertalev i looked into this and memory is the same both ways. the source is a skia bitmap but dart needs the pixels in our own buffer, so we hold both at the same time while copying = about 2x the bitmap no matter what. the rotated bitmap from createBitmap only lives for a moment and doesnt push the peak any higher than in place does. so in place doesnt save memory, it just swaps the native rotate for a jvm pixel loop thats about 2x slower on big raws (pixel 9a, 24mp: ~315ms vs ~625ms). only way to go lower is a zero copy bitmap handoff or stripe decoding the raw, and region decode doesnt do dng. so i think, keeping createBitmap is fine unless im missing something. but let me do some more research on if I can do any zero copy pointer reuse from skia bitmap. |
the exif rotate now happens in a small native pass (lock pixels + a tiled copy straight into the output buffer) so there's no second full bitmap. about 6x faster on big raws. also fixes orientation 5/7 which were swapped, and forces argb_8888 so high-bit-depth dng don't under-allocate. keeps the skia path as a fallback for odd formats.
|
@mertalev the zero copy route worked out. went native instead of the jvm loop. lock the skia bitmap pixels and rotate straight into our output buffer in c (added it to native_buffer since it already links jnigraphics), so no second bitmap like you wanted. and it ends up ~6x faster than createBitmap on raws, not slower (pixel 9a 24mp: ~330ms vs ~55ms). byte identical output, keeps the skia path as a fallback for odd formats. pushed it. |
| // "Load original" decodes a raw at full res, and the orientation pass then walks every pixel, so | ||
| // cap the decode resolution to keep that bounded on huge DNGs. This only trims pixels on very | ||
| // large raws - they still come out upright, just downsampled. | ||
| const val MAX_RAW_DECODE_PIXELS = 24_000_000L |
There was a problem hiding this comment.
I'd rather not set a cap since it affects fidelity.
There was a problem hiding this comment.
fair, and its worse than it looks, power of two so 50mp drops to 12.5. but i can't drop it, the rotate holds source + dest so an uncapped 100mp raw is about 770mb and 200mp about 1.5gb, ooms low ram phones. better to raise it to a 67mp total pixel cap (same as #29367) so every real raw stays full res and only giants trim. ok?
There was a problem hiding this comment.
The bug this is fixing is the orientation, right? Resolution is orthogonal to that. There are ways to lower the memory usage without reducing resolution, so I'd rather treat this as a separate issue. I've also tested 200MP on a device with 4GB RAM and it works fine.
| "rowBytes" to info[2].toLong() | ||
| ) | ||
| } | ||
| // Native path declined (unsupported config) -> rotate via Skia, then copy out. |
There was a problem hiding this comment.
good catch, the comment's misleading. i force 8888 before the native call so config can't be it, the only real return 0 is the dest malloc oom (and the skia fallback would oom too). i'll reword it to native alloc failed, or just drop the fallback. either works.
There was a problem hiding this comment.
I think we can just drop the fallback and consider it fatal.
| signal.throwIfCanceled() | ||
| val uri = ContentUris.withAppendedId(Images.Media.EXTERNAL_CONTENT_URI, id) | ||
| val handleRaw = Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q && isRawMime(uri) | ||
| val orientation = if (handleRaw) rawOrientation(uri) else ExifInterface.ORIENTATION_NORMAL |
There was a problem hiding this comment.
The ImageDecoder callback already gives you the header metadata, so you shouldn't need to hit the DB.
There was a problem hiding this comment.
ImageInfo is only size/mimetype/colorspace/isAnimated, no orientation (checked on device, a dng decodes sideways while a jpeg auto rotates, thats the bug), so the header can't give it. and it reads the file exif, not the db. cheaper would be the MediaStore ORIENTATION column, but its 0/missing for sideloaded raw so i'd keep exif as fallback. want that switch?
There was a problem hiding this comment.
I guess it depends on timing. Can you check how long it takes to read the EXIF the current way vs reading the DB? There is also the option of loading the encoded image to memory (which is relatively small), reading EXIF from it and decoding it. That minimizes IO so should presumably be the fastest.
2 participants
Description
raw photos (DNG etc) shot in portrait showed up sideways on android, both in the timeline grid and the full viewer. jpeg and heic were always fine.
turns out android's
ImageDecoderandloadThumbnail(API 29+) don't apply the EXIF orientation tag for raw files, so the decoded bitmap comes back unrotated. the jpeg/heic decoders rotate on their own, raw doesn't.so for raw we read the orientation tag and rotate the pixels ourselves. instead of allocating a second rotated bitmap (skia's createBitmap), we lock the decoded bitmap and rotate straight into the output buffer we already hand back to dart, in one native pass in C (added to the existing native_buffer lib, which already links jnigraphics). no extra full bitmap, and it's about 6x faster than the createBitmap approach on big raws. it runs on the background decode pool so it doesn't block the ui. the "load original" full-res decode is capped to ~24mp so memory stays bounded on big-sensor phones, which only trims pixels on huge raws (they still come out upright). non-8888 decodes (e.g. hdr dng) get converted to 8888 and rotated natively too, with the skia path kept only as a safety net if the native rotate ever fails (e.g. oom). jpeg/heic and the pre-29 paths are untouched. on-device (local) raw only.
fixes #24796
tested on a pixel 9a with real DNGs: orientation 6 (portrait) and 8 (front-cam) were sideways before, now upright in the grid, the viewer, and on load-original, matching the paired jpeg and byte-for-byte identical to the old skia rotate. all 8 EXIF orientations verified against the spec (cameras only emit 1/3/6/8 in practice). no jank, no native crash, no leak in logcat; jpeg/heic still display correctly.